Skip to content

configurable keymaps #136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jesses-code-adventures
Copy link

@jesses-code-adventures jesses-code-adventures commented May 1, 2025

Configurable Keymaps

This PR allows users to configure keymaps in the .opencode.json file with
the following layout:

{
    "keymaps": {
        "logs": {
            "keys": ["ctrl+w", "ctrl+e"],
            "keymap_display": "ctrl+w/e",
            "command_display": "look at my logs"
        }
    }
}

Although the configuration's a little verbose, it does allow users to
customize to their heart's content - happy to get rid of the display
configuration if it's unnecessary though.

I have ensured the same keys are displayed in the help window, but the
undisplayed keys are also configurable.

This PR was started after looking through @ritikpal1122 s work in #112, so
thanks to Ritik for getting started on this one.

@jesses-code-adventures
Copy link
Author

@kujtimiihoxha I just ran the test suite locally and realised this has broken a test or two - will fix these up and ping you when the PR is actually ready for review, but will do this tomorrow!

@ritikpal1122
Copy link

@jesses-code-adventures What i have to actually change in this? or will you help me by this ?

@kujtimiihoxha
Copy link
Collaborator

@jesses-code-adventures might be that the tests that are failing are not related to your PR just so you know, I have not really done a good job maintaining the tests.

@ritikpal1122 I think you can close your old PR for now seems like this implements the necessary keymaps using the current config, thanks a lot for the effort you have put into this feature.

@jesses-code-adventures
Copy link
Author

Ok nice one @kujtimiihoxha, I'll have a quick look today and see if it's anything my PR touches and let you know

@jesses-code-adventures
Copy link
Author

Hey @kujtimiihoxha, the failing test is in internal/llm/tools/ls_test.go, as we panic if the config isn't loaded when calling config.WorkingDirectory(). This test also seems to be failing on main, so it appears to be unrelated to this PR.

t.Run("handles empty path parameter", func(t *testing.T) {
  // For this test, we need to mock the config.WorkingDirectory function
  // Since we can't easily do that, we'll just check that the response doesn't contain an error message
  
  tool := NewLsTool()
  params := LSParams{
	  Path: "",
  }
  
  paramsJSON, err := json.Marshal(params)
  require.NoError(t, err)
  
  call := ToolCall{
	  Name:  LSToolName,
	  Input: string(paramsJSON),
  }
  
  response, err := tool.Run(context.Background(), call)
  require.NoError(t, err)
  
  // The response should either contain a valid directory listing or an error
  // We'll just check that it's not empty
  assert.NotEmpty(t, response.Content)
})

Let me know your preference, happy to try get the test working, remove it, or just leave as-is. Cheers!

@jesses-code-adventures
Copy link
Author

Maybe unrelated, but I also noticed when testing on a new machine that on both main and this branch, we throw an error if the user doesn't have a config file in $HOME/.opencode.json.

Would you like me to add the creation of a default config file if the user doesn't have one, or is this by design? Happy to add that in this PR or a separate one if desired.

@ritikpal1122
Copy link

Ok @kujtimiihoxha will revert or cancel my PR

but i want to contribute in more with @jesses-code-adventures Let ME know if i can join you

@kujtimiihoxha
Copy link
Collaborator

kujtimiihoxha commented May 2, 2025

Maybe unrelated, but I also noticed when testing on a new machine that on both main and this branch, we throw an error if the user doesn't have a config file in $HOME/.opencode.json.

This is probably a bug, we should not require that.

@kujtimiihoxha
Copy link
Collaborator

@jesses-code-adventures I think for now just remove the failing tests, I will need to do real testing of everything soon.

@jesses-code-adventures
Copy link
Author

no worries @kujtimiihoxha, have removed them now

@kujtimiihoxha
Copy link
Collaborator

@jesses-code-adventures looks like this needs a rebase, and there is a new global keymap ctrl+f for the file picker.

@jesses-code-adventures
Copy link
Author

@kujtimiihoxha no worries, changes have been applied

Copy link
Collaborator

@kujtimiihoxha kujtimiihoxha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small suggestion and I think this needs a rebase again.

Comment on lines +289 to +290
"keymap_display": "ctrl+w/e",
"command_display": "look at my logs"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"keymap_display": "ctrl+w/e",
"command_display": "look at my logs"
"help_key": "ctrl+w/e",
"help_description": "look at my logs"

I believe using help_* is a bit more descriptive here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants